-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check the Self-type of inherent associated constants #58353
Conversation
c468033
to
a7ae7a2
Compare
Removed the overlap with #58371 |
src/librustc_typeck/check/mod.rs
Outdated
@@ -2236,7 +2236,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
hir_id, def_id, substs, user_self_ty, self.tag(), | |||
); | |||
|
|||
if !substs.is_noop() { | |||
if !substs.is_noop() || user_self_ty.map_or(false, |ty| { | |||
ty.has_free_regions() || ty.has_projections() || ty.has_infer_types() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
~~Is there some justification for why these checks are enough?~~Found it
If there is, could you write it in a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if these checks are enough, shouldn't they be run on the pair (substs, ty)
? i.e.
let types = (substs, ty); // yes there are TypeFoldable impls for pairs and options
if types.has_free_regions() || types.has_projections() || types.has_infer_types() {
// ...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah I've seen a comment at https://github.com/rust-lang/rust/blob/a7ae7a21624952f5bfc6aaf88e53351385b89dd4/src/librustc_typeck/check/mod.rs#L2436-L2444
Maybe move the check there it to a separate function (needs_user_type_annotation
), while still doing the types
trick (where T: TypeFoldable<'tcx>
on that function, and then call it)?
r=me with the |
a7ae7a2
to
283ffcf
Compare
@bors r+ |
📌 Commit 283ffcf has been approved by |
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
⌛ Testing commit 283ffcf with merge df4ed96c920c6fe2ae1b95fac48321e46808be0b... |
@bors treeclosed=1000 |
💔 Test failed - checks-travis |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors retry -- apparently we can't clone the repo anymore on macOS |
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
…nts, r=arielb1 Check the Self-type of inherent associated constants r? @arielb1
Rollup of 16 pull requests Successful merges: - #58100 (Transition librustdoc to Rust 2018) - #58122 (RangeInclusive internal iteration performance improvement.) - #58199 (Add better error message for partial move) - #58227 (Updated RELEASES.md for 1.33.0) - #58353 (Check the Self-type of inherent associated constants) - #58453 (SGX target: fix panic = abort) - #58476 (Remove `LazyTokenStream`.) - #58526 (Special suggestion for illegal unicode curly quote pairs) - #58595 (Turn duration consts into associated consts) - #58609 (Allow Self::Module to be mutated.) - #58628 (Optimise vec![false; N] to zero-alloc) - #58643 (Don't generate minification variables if minification disabled) - #58648 (Update tests to account for cross-platform testing and miri.) - #58654 (Do not underflow after resetting unmatched braces count) - #58658 (Add expected/provided byte alignments to validation error message) - #58667 (Reduce Miri-related Code Repetition `like (n << amt) >> amt`) Failed merges: r? @ghost
r? @arielb1